add task solution fix#1549
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation is missing required handlers. The task explicitly states 'add success and error handlers for both promises', meaning each promise needs both a .then() handler AND a .catch() handler. Currently, promise1 only has .then() and is missing .catch(), while promise2 only has .catch() and is missing .then(). To fix this:
- Add a
.catch()handler topromise1that callsshowMessage(false) - Add a
.then()handler topromise2that callsshowMessage(true)
The .then() handler for promise2 would never execute in the current flow (since the promise rejects before user click), but it's still required per the specification and can simply log to the console or call showMessage(true).
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| document.body.appendChild(div); | ||
| } | ||
|
|
||
| promise1.then(() => showMessage('Promise was resolved!')); |
There was a problem hiding this comment.
Missing error handler for promise1. The requirement states 'add success and error handlers for both promises', so promise1 also needs a .catch() handler.
| } | ||
|
|
||
| promise1.then(() => showMessage('Promise was resolved!')); | ||
| promise2.catch(() => showMessage('Promise was rejected!', true)); |
There was a problem hiding this comment.
Missing success handler for promise2. The requirement states 'add success and error handlers for both promises', so promise2 also needs a .then() handler.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation correctly fulfills all the requirements specified in the task. Both promise1 and promise2 are properly set up with both success and error handlers, the click event listener on .logo correctly resolves promise1, and the 3-second timeout correctly rejects promise2. The message display logic correctly differentiates between success and error states by applying the appropriate classes and text content. All checklist items pass, and there are no critical issues or blockers that need to be addressed.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
https://yana-longstocking.github.io/js_promise_basic_DOM/